fix(ext/node): dns resolveAny with real ANY query, retry/maxTimeout support#33684
Closed
bartlomieju wants to merge 1 commit intomainfrom
Closed
fix(ext/node): dns resolveAny with real ANY query, retry/maxTimeout support#33684bartlomieju wants to merge 1 commit intomainfrom
bartlomieju wants to merge 1 commit intomainfrom
Conversation
…upport - Replace queryAny's 11 separate DNS queries with a single ANY query by adding RecordType::ANY support to op_dns_resolve. For ANY queries, each record's actual type is used for formatting, and a record_type field is included to let the JS layer distinguish untagged string variants (A vs NS vs PTR vs CNAME). - Implement retry logic in the DNS resolver: timeout doubles on each retry attempt, capped by the new maxTimeout option. - Distinguish Interrupted (explicit cancel - don't retry) from TimedOut (hickory timeout - retry if attempts remain). - Add maxTimeout option validation to the Resolver constructor. - Keep the short hickory timeout when cancel handle is present so background resolver tasks clean up quickly after cancellation. Enables 3 more Node.js compat tests: - parallel/test-dns-resolveany.js - parallel/test-dns-resolveany-bad-ancount.js - parallel/test-dns-resolver-max-timeout.js Closes #33577
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #33577.
Summary
queryAny's 11 separate DNS queries with a singleANYquery by addingRecordType::ANYsupport toop_dns_resolve. For ANY queries, each record's actual type is used for formatting, and arecord_typefield is included to let the JS layer distinguish untagged string variants (A vs NS vs PTR vs CNAME).maxTimeoutoption.Interrupted(explicit cancel - don't retry) fromTimedOut(hickory timeout - retry if attempts remain). This fixes the cancel regression from fix(ext/node): dns resolveAny with real ANY query, retry/maxTimeout support #33577 whereInterruptedwas retried, causingtest-dns-channel-cancel.jsto hang for 20s.maxTimeoutoption validation to theResolverconstructor (valid range:0to2^31-1).Enables 3 more Node.js compat tests:
parallel/test-dns-resolveany.jsparallel/test-dns-resolveany-bad-ancount.jsparallel/test-dns-resolver-max-timeout.jsTest plan
./x test-compat test-dns-resolveany- both pass (resolveany + bad-ancount)./x test-compat test-dns-resolver-max-timeoutpassestest-dns-channel-cancel.jspasses in ~96ms (no regression)test-dns-channel-timeout.jspassestools/format.jsandtools/lint.js --jspass